Skip to content

chore(build): fix all mypy errors in setup.py#17963

Open
vlad-scherbich wants to merge 2 commits into
mainfrom
vlad/fix-setup-py-mypy-errors
Open

chore(build): fix all mypy errors in setup.py#17963
vlad-scherbich wants to merge 2 commits into
mainfrom
vlad/fix-setup-py-mypy-errors

Conversation

@vlad-scherbich
Copy link
Copy Markdown
Contributor

@vlad-scherbich vlad-scherbich commented May 7, 2026 •
edited
Loading

Description

Fix a few lint errors in setup.py. Discovered when working on #17959.

Changes

  • Add proper type annotations to LibraryDownload class attributes (name, version, url_root, available_releases, translate_suffix), resolving var-annotated and assignment errors in subclasses.
  • Suppress cmake.CMAKE_BIN_DIR attr-defined errors (runtime-only attribute not declared in the cmake package stubs).
  • Suppress method-assign errors for intentional debug-instrumentation monkey-patching of CustomBuildExt methods.
  • Wrap src.suffix in bool() to fix return type mismatch in CMakeExtension.get_sources.

Testing

  • commit passed pre-commit hooks
  • CI

@cit-pr-commenter-54b7da
Copy link
Copy Markdown

cit-pr-commenter-54b7da Bot commented May 7, 2026

Codeowners resolved as

setup.py                                                                @DataDog/python-guild

@pr-commenter
Copy link
Copy Markdown

pr-commenter Bot commented May 7, 2026 •
edited
Loading

Benchmarks

Benchmark execution time: 2026-05-07 19:54:40

Comparing candidate commit e6b1683 in PR branch vlad/fix-setup-py-mypy-errors with baseline commit 7834861 in branch main.

Found 0 performance improvements and 8 performance regressions! Performance is the same for 586 metrics, 4 unstable metrics.

scenario:iast_aspects-re_expand_aspect

  • 🟥 execution_time [+253.666µs; +290.449µs] or [+7.283%; +8.339%]

scenario:iastaspects-casefold_noaspect

  • 🟥 execution_time [+25.944µs; +30.085µs] or [+8.651%; +10.032%]

scenario:iastaspects-strip_aspect

  • 🟥 execution_time [+60.971µs; +72.177µs] or [+22.573%; +26.722%]

scenario:iastaspects-upper_aspect

  • 🟥 execution_time [+50.606µs; +56.350µs] or [+17.452%; +19.432%]

scenario:iastaspectsospath-ospathbasename_aspect

  • 🟥 execution_time [+95.337µs; +101.764µs] or [+23.598%; +25.188%]

scenario:span-start

  • 🟥 execution_time [+1.283ms; +1.448ms] or [+8.293%; +9.357%]

scenario:telemetryaddmetric-1-count-metric-1-times

  • 🟥 execution_time [+201.490ns; +239.507ns] or [+9.718%; +11.552%]

scenario:tracer-small

  • 🟥 execution_time [+26.323µs; +28.737µs] or [+7.551%; +8.243%]

@vlad-scherbich vlad-scherbich added the changelog/no-changelog A changelog entry is not required for this PR. label May 7, 2026
@vlad-scherbich vlad-scherbich marked this pull request as ready for review May 7, 2026 19:27
@vlad-scherbich vlad-scherbich requested a review from a team as a code owner May 7, 2026 19:27
KowalskiThomas
KowalskiThomas reviewed May 11, 2026
View reviewed changes
Comment thread setup.py
Copy link
Copy Markdown
Contributor

@KowalskiThomas KowalskiThomas May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're fixing those errors, you might want to check the others as well

Image Image Image

Also I have mypy telling me some of those comments aren't used?

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@KowalskiThomas KowalskiThomas KowalskiThomas left review comments

@rithikanarayan rithikanarayan Awaiting requested review from rithikanarayan rithikanarayan is a code owner automatically assigned from DataDog/python-guild

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

changelog/no-changelog A changelog entry is not required for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@vlad-scherbich @KowalskiThomas